Skip to content

feat: Validate tiled ServiceAccount config at startup#1548

Open
tpoliaw wants to merge 3 commits into
opa-clientfrom
tiled-token-validation
Open

feat: Validate tiled ServiceAccount config at startup#1548
tpoliaw wants to merge 3 commits into
opa-clientfrom
tiled-token-validation

Conversation

@tpoliaw
Copy link
Copy Markdown
Contributor

@tpoliaw tpoliaw commented May 28, 2026

Ensure that the tiled auth configuration is valid and fail fast instead of when the first data is written.

@tpoliaw tpoliaw requested a review from a team as a code owner May 28, 2026 17:32
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (opa-client@5e8af18). Learn more about missing BASE report.

Additional details and impacted files
@@              Coverage Diff              @@
##             opa-client    #1548   +/-   ##
=============================================
  Coverage              ?   95.79%           
=============================================
  Files                 ?       44           
  Lines                 ?     3280           
  Branches              ?        0           
=============================================
  Hits                  ?     3142           
  Misses                ?      138           
  Partials              ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tpoliaw tpoliaw force-pushed the auth-checks branch 2 times, most recently from 5ed7d2e to 3b5b8a2 Compare May 29, 2026 10:27
@tpoliaw tpoliaw force-pushed the tiled-token-validation branch from ddf28f2 to 403846c Compare May 29, 2026 10:51
@tpoliaw tpoliaw changed the base branch from auth-checks to opa-client May 29, 2026 10:51
@tpoliaw tpoliaw force-pushed the tiled-token-validation branch 2 times, most recently from c23b4d7 to b575c2b Compare June 2, 2026 10:24
@tpoliaw tpoliaw force-pushed the tiled-token-validation branch from b575c2b to b6049c8 Compare June 3, 2026 10:49
Comment thread src/blueapi/config.py

class OpaConfig(BlueapiBaseModel):
root: HttpUrl = HttpUrl("http://localhost:8181")
tiled_service_account_check: str
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This changes only when the policy changes so we can have a default value for it here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends if we want to include diamond specific configuration as defaults? As far as I can see so far, there is nothing included so far. Would the services copier template be a better place to put the default values?

result: bool,
context: AbstractContextManager,
):
session.return_value.post = AsyncMock(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm writing this comment only once,But it applies to all the tests.

I personally think that pytest-aiohttp provide a better way of do this type of test. Creating so many mocks looks unnecessary/ clutters the code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how pytest-aiohttp would be used here? It appears to be targetted at testing webservers not mocking out responses.

@tpoliaw tpoliaw force-pushed the tiled-token-validation branch from b6049c8 to 63009c0 Compare June 5, 2026 14:27
@tpoliaw tpoliaw force-pushed the tiled-token-validation branch from 63009c0 to 9865265 Compare June 5, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants